PPL command expression implementation for geoip
#3228
Conversation
andy-k-improving
force-pushed
the
ft-ak-iplocation-cmd
branch
from
6138a91 to
16a8f8f
Compare
andy-k-improving
force-pushed
the
ft-ak-iplocation-cmd
branch
from
c972d60 to
b7acfe1
Compare
andy-k-improving
requested review from
ps48,
kavithacm,
derek-ho,
joshuali925,
dai-chen,
YANG-DB,
mengweieric,
Swiddis,
penghuo,
seankao-az,
MaxKsyunz,
Yury-Fridlyand,
anirudha,
forestmvey,
acarbonetto,
GumpacG,
ykmr1224,
LantaoJin and
noCharger
as code owners
|
As per the offline discussion, I have separated out the integration related changes into #3244, in order to minimise the diff. |
andy-k-improving
force-pushed
the
ft-ak-iplocation-cmd
branch
from
af264c2 to
590e405
Compare
docs/user/ppl/functions/geoip.rst
Outdated
| :local: | ||
| :depth: 1 | ||
|
|
||
| GEOIP |
There was a problem hiding this comment.
I thought GEOIP would go under ip.rst? Could we combine these files or is there a reason we need to split "Geo IP Address Functions" from "IP Address Functions".
There was a problem hiding this comment.
Not really, it makes more sense to merge both .rst.
There was a problem hiding this comment.
Actually my initial idea was that, since it's not possible to create the test dataSource (Not dataSet) in programmatically manner during the docTest run (Unlike integration-test I can setup the dataSource on Junit).
As the result, I will need to exclude this geoip.rst from the DocTest, and if this got merged into ip.rst, the original DoctTest validation from ip.rst will also be skipped.
core/src/main/java/org/opensearch/sql/expression/ip/GeoIPFunctions.java
Outdated
Show resolved
Hide resolved
integ-test/src/test/java/org/opensearch/sql/ppl/GeoIpFunctionsIT.java
Outdated
Show resolved
Hide resolved
integ-test/src/test/java/org/opensearch/sql/ppl/GeoIpFunctionsIT.java
Outdated
Show resolved
Hide resolved
integ-test/src/test/java/org/opensearch/sql/security/CrossClusterSearchIT.java
Outdated
Show resolved
Hide resolved
...rch/src/main/java/org/opensearch/sql/opensearch/planner/physical/OpenSearchEvalOperator.java
Outdated
Show resolved
Hide resolved
| Expression valueExpr = pair.getValue(); | ||
| ExprValue value; | ||
| if (valueExpr instanceof OpenSearchFunctionExpression openSearchFuncExpression) { | ||
| if ("geoip".equals(openSearchFuncExpression.getFunctionName().getFunctionName())) { |
There was a problem hiding this comment.
can we not fetchIpEnrichment within the geoip function itself?
There was a problem hiding this comment.
I don't think that is the feasible, as per the current design geoip function located in the core Gradle sub-module which is not mean to be storage engine specific implementation.
Hence for the alternative, I created OpenSearchEvalProcessor to serve a central registry for all eval OpenSearch specific operation.
WDYT?
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Co-authored-by: Andrew Carbonetto <[email protected]> Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
Signed-off-by: Andy Kwok <[email protected]>
andy-k-improving
force-pushed
the
ft-ak-iplocation-cmd
branch
from
96fdaea to
241ea63
Compare
Description
Introduce a new PPL command expression
geoip, to perform geo-spatial information lookup with the provided IPv4 || IPv6 addresses, result of the lookup is formatted into a tuple with attribute as key and location detail as value.In this particular setting, SQL plugin will act as a thin client, by relaying the IPEnrichment request to OpenSearch Geo-Spatial plugin, WITHIN the same cluster.
Detail implementation and interface that exposed on Geo-Spatial side can be found:
opensearch-project/geospatial#700
Internally this functionality is achieved by:
OpenSearchFunctionExpressionmarker to identify this is an expression has no default implement on other runtime (Ex: Prometheus)OpenSearchIndexin order to provide an OpenSearch specific handler forevaloperator and its expressions, when OS being used as the storage engine.During runtime, all eval expressions, will being passed to
OpenSearchIndex.visitEval( ), thenOpenSearchEvalOperatorclass will pick up the call, by evaluating allevalexpression as it is, and then handle all occasion ofOpenSearchFunctionExpressionseparately, by reading the function name and argument, and execute the appropriate business logic.Marker class
OpenSearchFunctionExpressionis being used in this case because the actual implementation require runtime OpenSearch client connectivity, howevercoremodule is mean to be generic, hence this workaround is being deployed, by tagging it asOpenSearchFunctionExpressiononcoreand only handle it on theopensearchCradle module .Related Issues
Resolves: #3037
Check List
--signoff.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.